Skip to content

Fix phpstan/phpstan#14250: trait.duplicateMethod is not reported#5181

Open
phpstan-bot wants to merge 8 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-y7tlrzi
Open

Fix phpstan/phpstan#14250: trait.duplicateMethod is not reported#5181
phpstan-bot wants to merge 8 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-y7tlrzi

Conversation

@phpstan-bot
Copy link
Collaborator

Summary

Duplicate method declarations within traits were not being reported with the trait.duplicateMethod identifier. The existing DuplicateDeclarationRule listens for InClassNode, but InClassNode is never emitted for trait declarations — NodeScopeResolver skips trait nodes entirely at declaration time (they are only processed when used by a class).

Changes

  • Created src/Rules/Classes/DuplicateTraitDeclarationRule.php — a new rule that implements Rule<InTraitNode> and checks for duplicate methods, properties, and constants within trait declarations
  • The rule mirrors the logic of DuplicateDeclarationRule but operates on InTraitNode (emitted when a trait is used by a class)
  • Registered at level 0 via #[RegisteredRule(level: 0)], same as the original rule
  • Added test class tests/PHPStan/Rules/Classes/DuplicateTraitDeclarationRuleTest.php
  • Added test data tests/PHPStan/Rules/Classes/data/bug-14250.php

Root cause

NodeScopeResolver::processStmtNodes() short-circuits trait declarations (line 967-968), returning immediately without emitting InClassNode. This is by design — traits are processed in the context of their using class via processTraitUse(). However, InTraitNode IS emitted during trait use processing, so a rule listening for InTraitNode can detect duplicate declarations within traits. No such rule existed before this fix.

Test

The regression test in bug-14250.php defines a trait with two methods named doSomething() and a class that uses the trait. The test expects one error: "Cannot redeclare method Bug14250\MyTrait::doSomething()." at line 11.

Fixes phpstan/phpstan#14250

… methods in traits

- Created DuplicateTraitDeclarationRule that listens for InTraitNode to detect duplicate methods, properties, and constants within trait declarations
- The existing DuplicateDeclarationRule only handles InClassNode, which is never emitted for traits (NodeScopeResolver skips trait declarations at line 967-968)
- InTraitNode is emitted when a trait is used by a class, allowing the new rule to check trait bodies for duplicates
- Added regression test in tests/PHPStan/Rules/Classes/data/bug-14250.php
Copy link
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be refactor with DuplicateDeclarationRule by creating some DuplicateDeclarationHelper

Also, this need more tests, for each possible trait duplicate content

phpstan-bot and others added 2 commits March 18, 2026 22:46
…clarationRule and DuplicateTraitDeclarationRule

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ts, properties, promoted properties, methods)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Collaborator Author

Done. Here's what I did to address the review:

1. Extracted DuplicateDeclarationHelper — A new helper class with a static checkClassLike() method containing all the shared duplicate-checking logic (constants, enum cases, properties, promoted properties, methods).

2. Refactored both rules to use the helper:

  • DuplicateDeclarationRule (for InClassNode) now delegates to DuplicateDeclarationHelper::checkClassLike()
  • DuplicateTraitDeclarationRule (for InTraitNode) does the same

3. Added comprehensive tests for each type of duplicate content in traits:

  • Duplicate constants (separate lines and comma-separated)
  • Duplicate properties (separate lines and comma-separated)
  • Duplicate methods (including case-insensitive duplicates)
  • Duplicate promoted constructor properties (traditional + promoted collision, and two promoted duplicates)

@VincentLanglet VincentLanglet requested a review from staabm March 19, 2026 08:19
@phpstan-bot
Copy link
Collaborator Author

Static analysis passes. The one test failure (RawErrorFormatterTest) is a pre-existing issue unrelated to this PR — it's about the raw error formatter now including identifiers in output, which is not related to the duplicate declaration changes.

The review from VincentLanglet is an approval with no additional changes requested. The previous feedback (extract helper, add more tests) was already addressed in the latest commits. Everything looks good — no further action needed.

class Qux
{
use TraitWithDuplicateMethods;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

append the following case in this test-file and add expected errors:

trait MyTrait1
{
	public function doSomething(): void
	{
	}
}

trait MyTrait2
{
	public function doSomething(): void
	{
	}
}

class FooWithMulitpleConflictingTraits
{
	use MyTrait1, MyTrait2;
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Added the test case with two separate traits (MyTrait1 and MyTrait2) each having a doSomething() method, used together by FooWithMultipleConflictingTraits. Since neither trait has duplicate declarations within itself, the DuplicateTraitDeclarationRule correctly reports no errors for this case (no false positives). The commit has been pushed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are missing errors when a class uses multiple traits with conflicting symbols

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed. Here's what I did to address the review:

Added cross-trait method collision detection — When a class uses multiple traits that define the same method without insteadof resolution, DuplicateDeclarationRule now reports an error. This mirrors PHP's fatal error: "Trait method X::method() has not been applied, because there are collisions with other trait methods on ClassName."

Changes:

  • DuplicateDeclarationHelper: Added checkTraitMethodCollisions() method that collects methods from each trait used by a class, respects insteadof adaptations and class-level method overrides, and reports unresolved conflicts
  • DuplicateDeclarationRule: Now calls checkTraitMethodCollisions() in addition to checkClassLike()
  • DuplicateDeclarationRuleTest: Added testBug14250TraitMethodCollisions (verifies the multi-trait case from bug-14250.php) and testTraitMethodCollisions (comprehensive test with unresolved collisions, insteadof resolution, class overrides, single trait, and partial collisions)

All tests pass (except the pre-existing RawErrorFormatterTest failure unrelated to this PR). Static analysis passes with no errors.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also add a test where the method is renamed when the trait is imported or the conflict is already resolved.

trait A {
    public function smallTalk() {
        echo 'a';
    }
    public function bigTalk() {
        echo 'A';
    }
}

trait B {
    public function smallTalk() {
        echo 'b';
    }
    public function bigTalk() {
        echo 'B';
    }
}

class Talker {
    use A, B {
        B::smallTalk insteadof A;
        A::bigTalk insteadof B;
    }
}

class Aliased_Talker {
    use A, B {
        B::smallTalk insteadof A;
        A::bigTalk insteadof B;
        B::bigTalk as talk;
    }
}

No error should be reported

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The background PHPStan task completed successfully (already confirmed above — no errors). Everything is pushed and ready.

phpstan-bot and others added 3 commits March 19, 2026 09:16
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a class uses multiple traits that define the same method without
`insteadof` resolution, PHP reports a fatal error. Add detection for
this case in DuplicateDeclarationRule via a new checkTraitMethodCollisions
helper method.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nd as alias

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the lint build

--------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------
  24 | ERROR | [x] Class \PHPStan\Rules\IdentifierRuleError should not be
     |       |     referenced via a fully qualified name, but via a use
     |       |     statement.
     |       |     (SlevomatCodingStandard.Namespaces.ReferenceUsedNamesOnly.ReferenceViaFullyQualifiedName)
 127 | ERROR | [x] Class \PHPStan\Rules\IdentifierRuleError should not be
     |       |     referenced via a fully qualified name, but via a use
     |       |     statement.
     |       |     (SlevomatCodingStandard.Namespaces.ReferenceUsedNamesOnly.ReferenceViaFullyQualifiedName)
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

…eference

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Collaborator Author

Fixed the lint errors by adding use PHPStan\Rules\IdentifierRuleError; and replacing the two fully qualified \PHPStan\Rules\IdentifierRuleError references with the short IdentifierRuleError form. All checks pass (lint, CS, tests, phpstan).

@phpstan-bot
Copy link
Collaborator Author

The review from VincentLanglet is an approval with no additional changes requested. All previous feedback (extract helper, add more tests, fix lint, add insteadof/alias tests) has already been addressed in the existing commits. No further action is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants